[`DETR`] Remove timm hardcoded logic in modeling files #29038

amyeroberts · 2024-02-15T13:48:22Z

What does this PR do?

Certain models use timm's create_model to load their backbone.

In future, all models should use load_backbone to create the backbone, removing the need for the conditional timm logic. Removing this from existing models isn't possible, because it changes the weight names for the backbone as the backbone is now loaded as a TimmBackbone class i.e. existing checkpoints wouldn't compatible.

This PR makes it possible to configure the timm backbone loaded completely through the model config, removing the hard-coded values in the modeling files. So, for users, it's the same as-if load_backbone was being used.

Before submitting

This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
Did you read the contributor guideline,
Pull Request section?
Was this discussed/approved via a Github issue or the forum? Please add a link
to it if that's the case.
Did you make sure to update the documentation with your changes? Here are the
documentation guidelines, and
here are tips on formatting docstrings.
Did you write any new necessary tests?

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

HuggingFaceDocBuilderDev · 2024-02-15T14:08:37Z

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

amyeroberts · 2024-03-08T14:57:08Z

src/transformers/models/deformable_detr/modeling_deformable_detr.py

@@ -141,23 +161,6 @@ def backward(context, grad_output):
        return grad_value, None, None, grad_sampling_loc, grad_attn_weight, None


-if is_scipy_available():


Moves these above the MultiScaleDeformableAttentionFunction definition - better matching library patterns

amyeroberts · 2024-03-08T14:58:37Z

src/transformers/models/conditional_detr/configuration_conditional_detr.py

-            raise ValueError("You can't specify both `backbone_kwargs` and `backbone_config`.")
-
-        if not use_timm_backbone:
+        if use_timm_backbone and backbone_kwargs is None:


These replicate the defaults that are used to load a timm backbone in the modeling file. This PR makes it possible to configure the timm backbone loaded, using the standard backbone API, the defaults here are for backwards compatibility

amyeroberts · 2024-03-08T15:02:57Z

src/transformers/models/conditional_detr/modeling_conditional_detr.py

@@ -354,17 +354,20 @@ def __init__(self, config):

        self.config = config

+        # For backwards compatibility we have to use the timm library directly instead of the AutoBackbone API
        if config.use_timm_backbone:


Unfortunately, we can't remove the timm logic here and use load_backbone instead. When using load_backbone a timm model is loaded as a TimmBackbone class. This means, the loaded weight names are different from using the create_model call here. For backwards compatibility - being able to load existing checkpoints - we need to leave as-is.

Instead - to be compatible with the backbone API and remove the hard-coding, we allow specifying of the backbone behaviour through backbone_kwargs

Ok, makes sense!

LysandreJik

Ok I think that makes sense! No problem for me, thanks for working on this @amyeroberts

LysandreJik · 2024-04-24T14:34:56Z

src/transformers/models/conditional_detr/modeling_conditional_detr.py

@@ -354,17 +354,20 @@ def __init__(self, config):

        self.config = config

+        # For backwards compatibility we have to use the timm library directly instead of the AutoBackbone API
        if config.use_timm_backbone:


Ok, makes sense!

LysandreJik · 2024-04-24T14:35:31Z

src/transformers/models/conditional_detr/modeling_conditional_detr.py

-            kwargs = {}
+            kwargs = getattr(config, "backbone_kwargs", {})
+            kwargs = {} if kwargs is None else kwargs.copy()
+            out_indices = kwargs.pop("out_indices", (1, 2, 3, 4))
+            num_channels = kwargs.pop("in_chans", config.num_channels)


I would maybe add a few comments here to explain what's happening for posterity

LysandreJik · 2024-04-24T14:35:44Z

src/transformers/models/deformable_detr/modeling_deformable_detr.py

-            kwargs = {}
+            kwargs = getattr(config, "backbone_kwargs", {})
+            kwargs = {} if kwargs is None else kwargs.copy()
+            out_indices = kwargs.pop("out_indices", (2, 3, 4) if config.num_feature_levels > 1 else (4,))
+            num_channels = kwargs.pop("in_chans", config.num_channels)
            if config.dilation:
-                kwargs["output_stride"] = 16
+                kwargs["output_stride"] = kwargs.get("output_stride", 16)


ArthurZucker

LGTM +1 on the comments for posterity! 🤗 sorry for being slow here

Co-authored-by: Arthur <[email protected]>

* Enable instantiating model with pretrained backbone weights * Clarify pretrained import * Use load_backbone instead * Add backbone_kwargs to config * Fix up * Add tests * Tidy up * Enable instantiating model with pretrained backbone weights * Update tests so backbone checkpoint isn't passed in * Clarify pretrained import * Update configs - docs and validation check * Update src/transformers/utils/backbone_utils.py Co-authored-by: Arthur <[email protected]> * Clarify exception message * Update config init in tests * Add test for when use_timm_backbone=True * Use load_backbone instead * Add use_timm_backbone to the model configs * Add backbone_kwargs to config * Pass kwargs to constructors * Draft * Fix tests * Add back timm - weight naming * More tidying up * Whoops * Tidy up * Handle when kwargs are none * Update tests * Revert test changes * Deformable detr test - don't use default * Don't mutate; correct model attributes * Add some clarifying comments * nit - grammar is hard --------- Co-authored-by: Arthur <[email protected]>

amyeroberts force-pushed the remove-timm-in-modeling-files branch from bc8d155 to 0a38d16 Compare February 19, 2024 15:04

amyeroberts force-pushed the remove-timm-in-modeling-files branch from 4d524f2 to 33c89b9 Compare March 7, 2024 19:11

amyeroberts commented Mar 8, 2024

View reviewed changes

amyeroberts changed the title ~~Remove timm in modeling files~~ [DETR] Remove timm hardcoded logic in modeling files Mar 8, 2024

amyeroberts mentioned this pull request Mar 8, 2024

New model support RTDETR #29077

Merged

9 tasks

amyeroberts requested a review from LysandreJik March 12, 2024 20:53

amyeroberts mentioned this pull request Mar 18, 2024

Adding grounding dino #26087

Merged

8 tasks

huggingface deleted a comment from github-actions bot Apr 8, 2024

amyeroberts force-pushed the remove-timm-in-modeling-files branch from dbc1355 to 51eb6d3 Compare April 8, 2024 14:11

amyeroberts requested review from ArthurZucker and removed request for LysandreJik April 8, 2024 14:12

LysandreJik approved these changes Apr 24, 2024

View reviewed changes

ArthurZucker approved these changes Apr 24, 2024

View reviewed changes

amyeroberts and others added 14 commits April 26, 2024 10:54

Enable instantiating model with pretrained backbone weights

5e8d40c

Clarify pretrained import

11ede69

Use load_backbone instead

e1067a4

Add backbone_kwargs to config

d19fc39

Fix up

59ba869

Add tests

899c9bd

Tidy up

6fc904d

Enable instantiating model with pretrained backbone weights

ad201dd

Update tests so backbone checkpoint isn't passed in

589007d

Clarify pretrained import

e593007

Update configs - docs and validation check

4a601dc

Update src/transformers/utils/backbone_utils.py

4506d08

Co-authored-by: Arthur <[email protected]>

Clarify exception message

cbc0f6a

Update config init in tests

89804ae

amyeroberts added 17 commits April 26, 2024 10:54

Add test for when use_timm_backbone=True

6300aef

Use load_backbone instead

6870317

Add use_timm_backbone to the model configs

aa3376c

Add backbone_kwargs to config

737fd0c

Pass kwargs to constructors

62d79f7

Draft

19fd92d

Fix tests

f9cbc01

Add back timm - weight naming

cffd51f

More tidying up

5bf5329

Whoops

7d4e93a

Tidy up

ac56450

Handle when kwargs are none

1c55822

Update tests

30d3232

Revert test changes

8183436

Deformable detr test - don't use default

61fe673

Don't mutate; correct model attributes

94309d9

Add some clarifying comments

80b32cc

amyeroberts force-pushed the remove-timm-in-modeling-files branch from 51eb6d3 to 80b32cc Compare April 26, 2024 11:00

nit - grammar is hard

5a9799c

amyeroberts merged commit aafa7ce into huggingface:main Apr 26, 2024
20 checks passed

amyeroberts deleted the remove-timm-in-modeling-files branch April 26, 2024 15:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[`DETR`] Remove timm hardcoded logic in modeling files #29038

[`DETR`] Remove timm hardcoded logic in modeling files #29038

amyeroberts commented Feb 15, 2024 •

edited

Loading

HuggingFaceDocBuilderDev commented Feb 15, 2024

amyeroberts Mar 8, 2024

amyeroberts Mar 8, 2024

amyeroberts Mar 8, 2024

LysandreJik Apr 24, 2024

LysandreJik left a comment

LysandreJik Apr 24, 2024

LysandreJik Apr 24, 2024

LysandreJik Apr 24, 2024

ArthurZucker left a comment

		@@ -141,23 +161,6 @@ def backward(context, grad_output):
		return grad_value, None, None, grad_sampling_loc, grad_attn_weight, None


		if is_scipy_available():

[DETR] Remove timm hardcoded logic in modeling files #29038

[DETR] Remove timm hardcoded logic in modeling files #29038

Conversation

amyeroberts commented Feb 15, 2024 • edited Loading

What does this PR do?

Before submitting

Who can review?

HuggingFaceDocBuilderDev commented Feb 15, 2024

amyeroberts Mar 8, 2024

Choose a reason for hiding this comment

amyeroberts Mar 8, 2024

Choose a reason for hiding this comment

amyeroberts Mar 8, 2024

Choose a reason for hiding this comment

LysandreJik Apr 24, 2024

Choose a reason for hiding this comment

LysandreJik left a comment

Choose a reason for hiding this comment

LysandreJik Apr 24, 2024

Choose a reason for hiding this comment

LysandreJik Apr 24, 2024

Choose a reason for hiding this comment

LysandreJik Apr 24, 2024

Choose a reason for hiding this comment

ArthurZucker left a comment

Choose a reason for hiding this comment

[`DETR`] Remove timm hardcoded logic in modeling files #29038

[`DETR`] Remove timm hardcoded logic in modeling files #29038

amyeroberts commented Feb 15, 2024 •

edited

Loading